Deprecate min_max and migrate to group sensor#167718
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to deprecate the min_max integration by warning on YAML usage via Repairs and migrating existing config entries to the group sensor helper instead.
Changes:
- Adds a Repairs issue for YAML-configured
min_maxsensors with a suggested convertedgroupsensor YAML snippet. - Updates the
min_maxconfig flow to immediately abort and direct users to usegroupinstead. - Introduces a config entry migration path intended to move
min_maxentries togroupsensor entries.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| homeassistant/components/min_max/yaml.yaml | Adds an example YAML conversion snippet (appears to be a temporary/scratch file). |
| homeassistant/components/min_max/strings.json | Adds abort reason text and a Repairs issue translation for YAML deprecation messaging. |
| homeassistant/components/min_max/sensor.py | Creates a Repairs issue for YAML configs and attempts to generate a replacement group sensor YAML snippet. |
| homeassistant/components/min_max/config_flow.py | Replaces the previous config flow/options flow with an abort-only flow directing users to group. |
| homeassistant/components/min_max/init.py | Adds async_migrate_entry intended to migrate config entries. |
Comments suppressed due to low confidence (1)
homeassistant/components/min_max/sensor.py:125
- Restore config-entry support (or ensure entries are migrated/removed before setup) because the sensor platform no longer defines
async_setup_entry, butmin_maxstill forwards config entries to the sensor platform inasync_setup_entry.
async def async_setup_platform(
hass: HomeAssistant,
config: ConfigType,
async_add_entities: AddEntitiesCallback,
discovery_info: DiscoveryInfoType | None = None,
) -> None:
"""Set up the min/max/mean sensor."""
entity_ids: list[str] = config[CONF_ENTITY_IDS]
name: str | None = config.get(CONF_NAME)
sensor_type: str = config[CONF_TYPE]
round_digits: int = config[CONF_ROUND_DIGITS]
unique_id = config.get(CONF_UNIQUE_ID)
await async_setup_reload_service(hass, DOMAIN, PLATFORMS)
await yaml_deprecation_notice(hass, config)
async_add_entities(
[MinMaxSensor(entity_ids, name, sensor_type, round_digits, unique_id)]
)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
homeassistant/components/group/init.py:56
- Drop the unused
CONF_IGNORE_NON_NUMERICimport to avoid failing linting for an unused symbol.
ATTR_ORDER,
ATTR_REMOVE_ENTITIES,
CONF_HIDE_MEMBERS,
CONF_IGNORE_NON_NUMERIC,
DATA_COMPONENT,
DOMAIN,
GROUP_ORDER,
REG_KEY,
)
| if ( | ||
| state := self.hass.states.get(entity_id) | ||
| ) is not None and state.state != STATE_UNKNOWN: | ||
| ) is not None and state.state not in (STATE_UNKNOWN, STATE_UNAVAILABLE): | ||
| raise ValueError("Only entities that haven't been loaded can be migrated") |
There was a problem hiding this comment.
We should fix this in that case in a separate PR since unknown also does not mean it's not loaded.
| config[CONF_IGNORE_NON_NUMERIC] = False | ||
| config[CONF_GROUP_TYPE] = SENSOR_DOMAIN | ||
|
|
||
| new_config_entry = ConfigEntry( |
There was a problem hiding this comment.
can't we use a normal config flow on the group domain to create the new entry, instead of adding it directly?
There was a problem hiding this comment.
That would be a bit difficult as we need to unload min_max and change the entity before it starts to ensure we keep history etc.
There was a problem hiding this comment.
This sort of thing would normally be in the group integration's config flow with a custom initial step for the min_max adoption, but considering we'll be removing the code after 6 months, this is maybe OK.
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
| if ( | ||
| state := self.hass.states.get(entity_id) | ||
| ) is not None and state.state != STATE_UNKNOWN: | ||
| ) is not None and state.state not in (STATE_UNKNOWN, STATE_UNAVAILABLE): | ||
| raise ValueError("Only entities that haven't been loaded can be migrated") |
There was a problem hiding this comment.
I already commented on this above, but I think another PR needs to fix that and look on the integration rather than an entity state (as unknown isn't always meaning an integration isn't loaded)
| config.pop(CONF_ROUND_DIGITS) | ||
| # Set group sensor defaults | ||
| config[CONF_HIDE_MEMBERS] = False | ||
| config[CONF_IGNORE_NON_NUMERIC] = False |
There was a problem hiding this comment.
I'd think we should migrate to group default (False), the user can always change that if they want.
| platform_config = config.copy() | ||
| platform_config[CONF_ENTITIES] = platform_config.pop(CONF_ENTITY_IDS) | ||
| platform_config.pop(CONF_ROUND_DIGITS) | ||
| platform_config.pop(CONF_PLATFORM) | ||
| if CONF_NAME not in platform_config: | ||
| platform_config[CONF_NAME] = f"{platform_config[CONF_TYPE]} sensor".capitalize() | ||
| yaml_config = yaml_util.dump(platform_config) | ||
| yaml_config = yaml_config.replace("\n", "\n ") | ||
| yaml_config = "```yaml\nsensor:\n - platform: group\n " + yaml_config + "\n```" |
| async def test_deprecation_warning( | ||
| hass: HomeAssistant, issue_registry: ir.IssueRegistry | ||
| ) -> None: | ||
| """Test the min sensor with a default name.""" |
There was a problem hiding this comment.
Yes, but I'll leave that for now until we have a second opinion as there could potentially come up more things.
abmantis
left a comment
There was a problem hiding this comment.
I think this is ok, but would like another pair of eyes since I am not familiar with the migration as done here.
| async def test_deprecation_warning( | ||
| hass: HomeAssistant, issue_registry: ir.IssueRegistry | ||
| ) -> None: | ||
| """Test the min sensor with a default name.""" |
There was a problem hiding this comment.
Please move the entity registry changes to a separate PR with a clear motivation for the change as well as updated tests.
| "round_digits": "Controls the number of decimal digits in the output when the statistics characteristic is mean, median or sum." | ||
| }, | ||
| "description": "Create a sensor that calculates a min, max, mean, median or sum from a list of input sensors.", | ||
| "description": "Min/Max helper has been deprecated, please use the Group helper instead.", |
There was a problem hiding this comment.
Is this visible considering we immediately abort the flow?
There was a problem hiding this comment.
No, I will remove it
There was a problem hiding this comment.
Have to keep it as the translation script requires it. It will anyhow be removed in 6 months
| if ( | ||
| state := self.hass.states.get(entity_id) | ||
| ) is not None and state.state != STATE_UNKNOWN: | ||
| ) is not None and state.state not in (STATE_UNKNOWN, STATE_UNAVAILABLE): | ||
| raise ValueError("Only entities that haven't been loaded can be migrated") |
| ) -> RepairsFlow: | ||
| """Create flow.""" | ||
| if data and (entry_id := data.get("entry_id")): | ||
| entry_id = cast(str, entry_id) | ||
| entry = hass.config_entries.async_get_entry(entry_id) | ||
| if TYPE_CHECKING: | ||
| assert entry | ||
| return MigrateToGroupSensorFlow(entry) | ||
|
|
||
| return ConfirmRepairFlow() |
Breaking change
Proposed change
Deprecate the
min_maxintegrationNeeds: #171773
Config entry repair:

YAML config repair:

Start new min/max helper:

Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: